Skip to content

fix: apply native reasoning effort to harnesses - #54

Open
vincentkoc wants to merge 2 commits into
mainfrom
fix/native-reasoning-effort-parity
Open

fix: apply native reasoning effort to harnesses#54
vincentkoc wants to merge 2 commits into
mainfrom
fix/native-reasoning-effort-parity

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

Propagates the native eval plan's reasoning effort into OpenClaw, Hermes,
Codex, and Claude Code client controls.

Why?

Fixes #53. Without this, a run can be labeled high while OpenClaw executes
with thinking disabled, invalidating cross-harness quality and efficiency
comparisons.

Changes

  • carry reasoning_effort in RunSpec and the run manifest
  • map effort to each native harness's supported control
  • translate ShellBench's canonical xhigh to Claude Code's max
  • preserve existing harness defaults when effort is unspecified
  • test low, medium, high, and xhigh across all four harnesses

Tests

  • 114 focused native runner, proxy, and fleet tests pass
  • focused Ruff check passes
  • fresh Codex autoreview reports no actionable findings
  • all GitHub CI and CodeQL checks pass

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 31, 2026, 6:31 AM ET / 10:31 UTC.

ClawSweeper review

What this changes

This PR carries a native evaluation plan’s reasoning-effort value through run specifications and manifests, then maps it to the OpenClaw, Hermes, Codex, and Claude Code client controls while retaining harness defaults when no effort is specified.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep this PR open for maintainer review. The branch addresses the paired benchmark-validity bug with focused propagation and coverage, but the four mappings depend on versioned external harness contracts that source-level command assertions do not prove.

Likely related people: vincentkoc is the available routing candidate as the recent native-evaluation contributor and author of the paired report; confidence is medium because current-main feature-history inspection was unavailable.

Priority: P1
Reviewed head: 887c2597c1e6ef7ad369cd9a72c800b8fb52ffeb
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused implementation and generated-command coverage are solid, with one remaining maintainer-visible external-client compatibility check.
Proof confidence 🌊 off-meta tidepool Not applicable: The PR author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; however, redacted runtime output from the pinned clients would materially reduce the compatibility risk.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The PR author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; however, redacted runtime output from the pinned clients would materially reduce the compatibility risk.
Evidence reviewed 6 items RunSpec propagation: The branch adds an optional reasoning-effort field to RunSpec and populates it while building matrix plans, providing one value for command construction and manifest recording.
Native control mappings: The branch replaces OpenClaw’s fixed thinking-off argument and adds conditional Hermes, Codex, and Claude Code mappings, preserving existing defaults when the plan omits effort.
Manifest and CLI transport: The branch accepts canonical low/medium/high/xhigh CLI values, puts the resolved value in RunSpec, and records that same value in the run manifest.
Findings None None.
Security None None.

How this fits together

ShellBench turns an evaluation plan into a RunSpec, then renders harness-specific commands and configuration for benchmark runs. Those runs produce manifests and traces that are later compared across harnesses, so their reasoning mode must match the planned setting.

flowchart LR
  A[Evaluation plan] --> B[Run specification]
  B --> C[Harness command builder]
  C --> D[OpenClaw and Hermes controls]
  C --> E[Codex and Claude Code controls]
  D --> F[Native benchmark run]
  E --> F
  F --> G[Run manifest and comparison data]
Loading

Decision needed

Question Recommendation
Should this MEMBER-authored PR be merged after maintainers verify the four native client controls against the benchmark images and pinned client versions? Verify pinned harness contracts: Run each harness at the supported effort values in the benchmark environment and merge after redacted output confirms the client accepted and applied the intended setting.

Why: The patch is structurally focused and its tests cover generated commands, but whether those commands are accepted and honored is an external runtime-contract decision that source inspection cannot settle.

Before merge

  • Resolve merge risk (P1) - Merging changes the effective reasoning behavior of existing native benchmark plans; generated-command tests do not establish that the exact OpenClaw, Hermes, Codex, and Claude Code versions pinned in benchmark environments accept and apply every requested value, especially the Claude Code xhigh to max translation.
  • Complete next step (P2) - This MEMBER-authored PR needs maintainer confirmation of the pinned native-client contracts before merge; no narrow additional code repair is established by the available evidence.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Harness coverage 4 harnesses changed The patch changes the effective execution controls for every native benchmark harness.
Mapping assertions 16 effort/harness cases added The parametrized test checks four canonical effort values across four harness command builders.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #53
Summary: This PR is the explicit candidate fix for the open report about native harnesses failing to apply planned reasoning effort consistently.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Verify native client behavior first (recommended)
    Capture redacted live output from the pinned OpenClaw, Hermes, Codex, and Claude Code clients showing that each configured effort is accepted and takes effect before merging.
  2. Merge with explicit version ownership
    Accept the remaining risk if maintainers intentionally own the current pinned-client contract and will update mappings when a harness CLI changes.

Technical review

Best possible solution:

Validate each canonical effort against the actual pinned native client versions with a redacted live run or logs, then merge the focused transport and mapping change if all four clients report the intended effective setting.

Do we have a high-confidence way to reproduce the issue?

No high-confidence live current-main reproduction is available in the hydrated evidence. The paired report describes a concrete trace mismatch, and the baseline command shown by this PR hard-codes OpenClaw thinking off, so the source path is reproducible but needs runtime confirmation.

Is this the best way to solve the issue?

Unclear. Propagating the existing plan value through each native harness is the narrowest apparent repair, but live proof against the pinned client versions is needed to confirm the selected controls and Claude Code translation are the right contracts.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 884dd1bb5511.

Labels

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; however, redacted runtime output from the pinned clients would materially reduce the compatibility risk.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P1: A mismatch between planned and effective reasoning mode can invalidate current cross-harness quality, latency, and efficiency comparisons.
  • merge-risk: 🚨 compatibility: The change relies on OpenClaw, Hermes, Codex, and Claude Code CLI/configuration contracts and alters existing benchmark command behavior.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; however, redacted runtime output from the pinned clients would materially reduce the compatibility risk.

Evidence

What I checked:

  • RunSpec propagation: The branch adds an optional reasoning-effort field to RunSpec and populates it while building matrix plans, providing one value for command construction and manifest recording. (scripts/native_eval/models.py:32, 887c2597c1e6)
  • Native control mappings: The branch replaces OpenClaw’s fixed thinking-off argument and adds conditional Hermes, Codex, and Claude Code mappings, preserving existing defaults when the plan omits effort. (scripts/native_eval/harnesses.py:532, 887c2597c1e6)
  • Manifest and CLI transport: The branch accepts canonical low/medium/high/xhigh CLI values, puts the resolved value in RunSpec, and records that same value in the run manifest. (scripts/native_eval/run_job.py:270, 887c2597c1e6)
  • Focused mapping coverage: The added parametrized test covers four canonical effort values across all four harnesses and separately asserts default preservation when effort is absent. (tests/test_native_eval_runner.py:681, 887c2597c1e6)
  • Paired issue relationship: The PR body and timeline identify the open report as the source problem; its stated OpenClaw mismatch is directly addressed by the changed fixed --thinking off command. (scripts/native_eval/harnesses.py:617, 887c2597c1e6)
  • Branch provenance: The second branch commit specifically normalizes ShellBench xhigh to Claude Code max, matching the compatibility-sensitive translation described in the PR. (scripts/native_eval/harnesses.py:822, 887c2597c1e6)

Likely related people:

  • vincentkoc: Authored the paired native-harness reasoning-effort report and the two commits implementing its proposed command/configuration mappings; GitHub identifies the author as a repository MEMBER. (role: recent native-evaluation contributor; confidence: medium; commits: 98301b240507, 887c2597c1e6; files: scripts/native_eval/harnesses.py, scripts/native_eval/models.py, scripts/native_eval/run_job.py)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Provide redacted live output from the pinned OpenClaw, Hermes, Codex, and Claude Code clients showing the effective setting for the canonical effort values.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (8 earlier review cycles)
  • reviewed 2026-07-29T16:06:36.439Z sha 98301b2 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-29T16:20:31.880Z sha 887c259 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-29T16:27:52.992Z sha 887c259 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-29T21:18:42.277Z sha 887c259 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T07:39:25.377Z sha 887c259 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T18:59:39.928Z sha 887c259 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T04:05:50.323Z sha 887c259 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T07:35:57.529Z sha 887c259 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot mentioned this pull request Jul 29, 2026
Open
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 29, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review July 29, 2026 16:23
@vincentkoc
vincentkoc requested a review from a team as a code owner July 29, 2026 16:23
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Native harnesses do not apply planned reasoning effort consistently

1 participant

@vincentkoc